[feat] Add database constraints for validation annotations #1085
Conversation
rubenvanassche
left a comment
rubenvanassche
left a comment
There was a problem hiding this comment.
Looking great, few changes required and then this one is good to go!
| ) { | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Can you add a list here with the possible constraints?
src/Attributes/Validation/Exists.php
Outdated
| $constraints = is_array($this->where) ? $this->where : [$this->where]; | ||
|
|
||
| foreach ($constraints as $constraint) { | ||
| match (true) { | ||
| $constraint instanceof Closure => $rule->where($constraint), | ||
| $constraint instanceof WhereConstraint => $rule->where(...$constraint->toArray()), | ||
| $constraint instanceof WhereNotConstraint => $rule->whereNot(...$constraint->toArray()), | ||
| $constraint instanceof WhereNullConstraint => $rule->whereNull(...$constraint->toArray()), | ||
| $constraint instanceof WhereNotNullConstraint => $rule->whereNotNull(...$constraint->toArray()), | ||
| $constraint instanceof WhereInConstraint => $rule->whereIn(...$constraint->toArray()), | ||
| $constraint instanceof WhereNotInConstraint => $rule->whereNotIn(...$constraint->toArray()), | ||
| default => throw new InvalidArgumentException('Each where item must be a DatabaseConstraint or Closure'), | ||
| }; | ||
| } |
There was a problem hiding this comment.
Would it be possible to move this to a trait since we duplicate the code?
| public readonly mixed $column, | ||
| public readonly mixed $value = null, |
There was a problem hiding this comment.
I know indeed laravel has a mixed type but they provide typehints, let's use those. For value mixed is allright since it probably is really mixed. Another thing I would like to see is support for ExternalReference since we can dynamically replace the value which might be useful.
There was a problem hiding this comment.
Good suggestion! I've updated the PR to allow the use of ExternalReference. I also added types to the database constraints to the ones that I found made sense.
I took the type suggestions from https://api.laravel.com/docs/11.x/Illuminate/Validation/Rules/DatabaseRule.html
I kept the $value of the WhereConstraint and WhereNotConstraint as mixed, like you said they probably really are mixed.
The type suggestion for whereNot does not include null, but that is supported.
This would otherwise error:
new WhereNotConstraint('deleted_at', null)Which is not a big deal, since you can use WhereNullConstraint instead, but it makes me think there are other inputs which are not in the type but are supported
…ature/annotation-validation-database-constraints
…ypes to DatabaseConstraints
…nstraints to docs
2 participants
Currently when using annotations with database rules like
ExistsandUnique, it is not possible to include database constraints.This will throw a php error:
This feature would allow using database constraints like so:
After testing this has been working great for us, happy with any feedback
One thing I'm not a 100% sure about:
DatabaseConstraintimplementations aremixedsince the Laravel methods are untyped themselves. Is this the best way?